-
Notifications
You must be signed in to change notification settings - Fork 1.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Rebase #7223: improve contracts error message #7319
Conversation
Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: Kyle Kent.
|
9cb5b3f
to
5d58707
Compare
5d58707
to
be78a7d
Compare
row = [yaml_col["name"], "", yaml_col["data_type"], "missing in definition"] | ||
mismatches += [dict(zip(column_names, row))] | ||
|
||
mismatches_sorted = sorted(mismatches, key=lambda d: d["column_name"]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💯✨
core/dbt/exceptions.py
Outdated
# if name matches | ||
if sql_col["name"] == yaml_col["name"]: | ||
# if type matches | ||
if sql_col["formatted"] == yaml_col["formatted"]: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for my own understanding - any reason not to do the comparison on data_type at this point?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@MichelleArk Good point! Since we're already comparing columns with the same name, might as well compare just the data_type
instead of the formatted
value.
This was just how Kyle had it, and it does match the logic here performing the functional comparison.
Thanks @jtcohen6 ! |
Nice work @kentkr ! 🏆 |
resolves #7209
rebases #7223
Description
@kentkr did the heavy lifting here! I just added a few small touch-ups:
agate
for structured representation, and to print to stdout in a markdown tableChecklist
I have opened an issue to add/update docs, or docs changes are not required/relevant for this PRchangie new
to create a changelog entry